Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search registries with an empty query #1444

Closed
wants to merge 2 commits into from

Conversation

umohnani8
Copy link
Member

Adds functionality to search registries implementing the v2
endpoint with an empty query, that is the results will be
all the available images on the registries.
If this is tried with a v1 registry an error will occur.
To search a whole registry, there needs to be a trailing slash
at the end, i.e podman search registry.fedoraproject.org/.

Fixes #1203

Signed-off-by: Urvashi Mohnani umohnani@redhat.com

@umohnani8
Copy link
Member Author

@rhatdan @mheon PTAL

@baude
Copy link
Member

baude commented Sep 12, 2018

bot, retest this please

@@ -345,6 +345,9 @@ func matchesOfficialFilter(filter searchFilterParams, result docker.SearchResult
}

func getRegistry(image string) (string, error) {
if image[len(image)-1] == '/' {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use strings.HasSuffix and strings.TrimSuffix here to be absolutely clear what's going on?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! Fixed

@mheon
Copy link
Member

mheon commented Sep 12, 2018

LGTM

@umohnani8
Copy link
Member Author

bot, retest this please

1 similar comment
@umohnani8
Copy link
Member Author

bot, retest this please

@umohnani8
Copy link
Member Author

Tests are finally green!

@@ -13,7 +13,9 @@ The user can specify which registry to search by prefixing the registry in the s
**registires.search** table in the config file - **/etc/containers/registries.conf**.
The number of results can be limited using the **--limit** flag. If more than one registry
is being searched, the limit will be applied to each registry. The output can be filtered
using the **--filter** flag.
using the **--filter** flag. To get all available images in a registry without a specific
query, the user can just enter the registry name with a trailing (example **registry.fedoraproject.org/**).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with a trailing ? I think you want '/' or "slash" here..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

using the **--filter** flag.
using the **--filter** flag. To get all available images in a registry without a specific
query, the user can just enter the registry name with a trailing (example **registry.fedoraproject.org/**).
Note, searching without a query will only work for registries that implement the v2 API.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Down below you say the search only works for v2, but here you're saying it works for v1, but only the query works on v2? I'm a bit confused.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded a bit.

@@ -13,7 +13,9 @@ The user can specify which registry to search by prefixing the registry in the s
**registires.search** table in the config file - **/etc/containers/registries.conf**.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

registries.search not registires.search

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@@ -13,7 +13,9 @@ The user can specify which registry to search by prefixing the registry in the s
**registires.search** table in the config file - **/etc/containers/registries.conf**.
The number of results can be limited using the **--limit** flag. If more than one registry
is being searched, the limit will be applied to each registry. The output can be filtered
using the **--filter** flag.
using the **--filter** flag. To get all available images in a registry without a specific
query, the user can just enter the registry name with a trailing (example **registry.fedoraproject.org/**).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK you mention query here, but I don't see query prior. In the command synopsis term is mentioned but no reference to what that is here. Should term be changed to query?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I think I got it. a query would look like registry.fedoraproject.org/fedora vs registry.fedoraproject.org. Are there any wild cards allowed? i.e. registry.fedoraproject.org/f* to get all the images that start with f? Might be nice to expound on that and/or have an example or two. Also 'term' in the synopsis isn't explained anywhere and I think you're allowing multiple registries to be handed in no? I don't recall the syntax, but if multiples are allowed I think that needs to change to something like [term, ...] but would have to look it up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we don't allow multiple registries to be handed in. If you just do podman search image then it will try all the registries from your registries.conf file. If you do podman search docker.io/image it will only search docker.io for image. And now if you do podman search myregistry/, it will return all the images it finds in the registry if it is a v2 registry.

Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the vendoring to be done in a separate PR as I don't think this PR depends on it, but shrug.

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably facab2a) made this pull request unmergeable. Please resolve the merge conflicts.

Picks up changes made to authentication for registry search.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
Adds functionality to search registries implementing the v2
endpoint with an empty query, that is the results will be
all the available images on the registries.
If this is tried with a v1 registry an error will occur.
To search a whole registry, there needs to be a trailing slash
at the end, i.e `podman search registry.fedoraproject.org/`.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
@umohnani8
Copy link
Member Author

@TomSweeneyRedHat the vendor is important here as it implements the podman search myregistry/ case in the containers/image side.

@umohnani8
Copy link
Member Author

bot, retest this please

@rhatdan
Copy link
Member

rhatdan commented Sep 13, 2018

@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit bbcfa3b has been approved by rhatdan

@rh-atomic-bot
Copy link
Collaborator

⚡ Test exempted: pull fully rebased and already tested.

rh-atomic-bot pushed a commit that referenced this pull request Sep 13, 2018
Adds functionality to search registries implementing the v2
endpoint with an empty query, that is the results will be
all the available images on the registries.
If this is tried with a v1 registry an error will occur.
To search a whole registry, there needs to be a trailing slash
at the end, i.e `podman search registry.fedoraproject.org/`.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>

Closes: #1444
Approved by: rhatdan
@TomSweeneyRedHat
Copy link
Member

@umohnani8 thanks for the revisions, I like what you did! 👍

@umohnani8 umohnani8 deleted the search branch October 27, 2020 14:27
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants